state-manager/app/controller/executed_state.py (5)

31-31: Pass identifiers to background tasks instead of full document instances

Handing a full State document to BackgroundTasks can introduce stale-data issues, heavier memory usage, and accidental coupling. Prefer passing state.id (and re-fetch inside the task), which also lets you drop the extra read after insert_many.

Apply this diff (if create_next_state supports fetching by id):

-            background_tasks.add_task(create_next_state, state)
+            background_tasks.add_task(create_next_state, state.id)

-            background_tasks.add_task(create_next_state, state)
+            background_tasks.add_task(create_next_state, state.id)

-                for inserted_state in inserted_states:
-                    background_tasks.add_task(create_next_state, inserted_state)
+                for inserted_state in inserted_states:
+                    background_tasks.add_task(create_next_state, inserted_state.id)

If create_next_state currently expects a State instance, I can provide a companion patch to refactor it to accept an id and perform a fresh lookup.

Also applies to: 38-38, 66-66

---

55-63: Skip re-query when scheduling background work (paired with id-passing)

If you switch to passing ids to the background task, you can avoid the extra find(In(State.id, inserted_ids)) round-trip altogether, scheduling tasks directly from inserted_ids.

Proposed adjustment:

-            if len(new_states) > 0:
-                inserted_ids = (await State.insert_many(new_states)).inserted_ids
-
-                inserted_states = await State.find(
-                    In(State.id, inserted_ids)
-                ).to_list()
-                
-                if len(inserted_states) != len(new_states):
-                    raise RuntimeError(f"Failed to insert all new states. Expected {len(new_states)} states, but only {len(inserted_states)} were inserted")
-
-                for inserted_state in inserted_states:
-                    background_tasks.add_task(create_next_state, inserted_state)
+            if new_states:
+                inserted_ids = (await State.insert_many(new_states)).inserted_ids
+                if len(inserted_ids or []) != len(new_states):
+                    raise RuntimeError(f"Failed to insert all new states. Expected {len(new_states)} states, but only {len(inserted_ids or [])} were inserted")
+                for inserted_id in inserted_ids:
+                    background_tasks.add_task(create_next_state, inserted_id)

---

26-39: Reduce duplication between empty/non-empty outputs branches

Both branches set status=EXECUTED, save, and enqueue create_next_state. Consider consolidating to reduce duplication and potential drift.

High-level idea:

• Compute outputs = body.outputs[0] if body.outputs else {}
• Set state.outputs = outputs, state.status = EXECUTED
• Save once, add one background task
• Then handle body.outputs[1:] for additional states

I can provide a concrete patch if you’d like.

---

70-72: Preserve traceback: use bare raise and log exception with stack

raise e can obscure the original traceback context. Prefer raise to rethrow the same exception and use an exception-aware logger to include the stack trace.

Apply this diff:

-    except Exception as e:
-        logger.error(f"Error executing state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id, error=e)
-        raise e
+    except Exception as e:
+        logger.exception(f"Error executing state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id, error=e)
+        raise

If LogsManager lacks exception, use logger.error(..., exc_info=True).

---

26-31: Add unit tests to guard the new parents semantics

We should assert that:

• state.parents is not mutated in-place after execution.
• New states created for outputs[1:] have parents equal to the original mapping, without any added self entry.

I can draft tests mirroring test_executed_state_empty_outputs to validate these invariants. Want me to open a testing PR or push commits to this one?

Also applies to: 34-39, 41-53